-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hoist boundary check code to the out of loop #10
Conversation
Thank you for the PR. I agree that boundary check should not be performed for every byte, I am basically fine to merge the changes. Before that, would you mind making following changes so that the diff would be minimal?
It would be great if you could make such changes so that the diff of gets limited to the topic of the branch (i.e. only change the I would appreciate it if you could either open a separate PR or a separate commit within this PR to change other things in case you are convinced that my understanding described in the above listing is wrong. |
cd28c9c
to
134c194
Compare
@kazuho
|
Thanks for the changes. I think we are almost complete. Please consider the nit-picking comments on the commit. |
Thank you for your comment. But we cannot modify the code as above because it breaks a compatibility with
|
I tried to reconsider about above comments and I updated the branch.
|
Hmm. Let me see. Thought that it was possible to not add any local vars... |
Sorry for the fuss. The proposed code seems optimal. Merged to master with little tweaks. |
@kazuho |
Current implementation of sprintf(..., "%x",...) always execute boundary check.
(e.g. https://github.com/h2o/qrintf/blob/master/share/qrintf/qrintf.h#L1321)
This pull request hoists this check to the out of loop to simplify main code of hexadecimal to string conversion.